Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 <listbox/> with transtion open on "enter" click #146

Merged

Conversation

far-fetched
Copy link
Contributor

@far-fetched far-fetched commented May 27, 2022

What changed and why:

Currently when using <Listbox/> component with composition with <Transtion/> it is not possible to use "Enter" key to open listbox.

Video from demo (try yourself):

simplescreenrecorder-2022-05-27_12.14.46.mp4

Fix:

listbox with transition suffers from race condition. When “enter” click, handler for keyPress is called – is sets flag isOpen to true.
Then after a while, the handler onButtonClick is also fired – this handler toggles isOpen property – so it reverts isOpen to false.
Everything above is executed on one “enter” click.

link to failed test.

</Listbox>`);

getListboxButton()?.focus();
userEvent.keyboard('{Enter}');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, triggerEvent from ember-helpers does not simulate "Enter" like real browser. Only one handler is executed, thus this test would pass even without a fix for the relevant issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the testing library util here do the whole series of events that occur when pressing keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, similar emulation we used with dialog test for tab press.

@far-fetched far-fetched force-pushed the fix-listbox-open-on-enter-click branch from 917854b to 4c9033f Compare June 1, 2022 06:49
@NullVoxPopuli
Copy link
Collaborator

Thanks for doing this!!

@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Jun 2, 2022
@NullVoxPopuli NullVoxPopuli merged commit acad1ed into GavinJoyce:master Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants